-
-
Notifications
You must be signed in to change notification settings - Fork 722
Add buildInfo metadata support #4509
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
| buildinfo_content += "dep\t{}\t{}\n".format(dep_path, dep_version) | ||
|
|
||
| go.actions.write( | ||
| output = buildinfo_file, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we track info as depsets (see comment above), we could materialize that into a file at execution time using TemplateDict or Args. I can take over that part if you want.
| def _buildinfo_aspect_impl(target, ctx): | ||
| """Collects package_info metadata from dependencies. | ||
| Following bazel-contrib/supply-chain gather_metadata pattern, this checks: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Yannic I haven't followed this repo lately, are there any bits that rulesets can reuse rather than adapt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we need to support WORKSPACE, I don't think we can yet depend on the supply-chain tools as that is currently bzlmod only which is why I have duplicated this logic in here.
| """ | ||
| direct_versions = [] | ||
|
|
||
| # Approach 1: Check package_metadata common attribute (Bazel 5.4+) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We only support Bazel 6 and even that will be dropped soon. We could skip the fallback.
| # No metadata at all, return empty provider | ||
| return [BuildInfoMetadata(version_map = depset([]))] | ||
|
|
||
| if not direct_versions and len(transitive_depsets) == 1: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This optimization is unnecessary, the depset constructor already does this (and more)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed
| attr_aspects = ["*"], # Traverse all attributes (supply-chain pattern) | ||
| provides = [BuildInfoMetadata], | ||
| # Apply to generated targets including package_info | ||
| apply_to_generating_rules = True, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would think that we don't want this (e.g. don't traverse code generators that produce .go files in sources), but I may be missing a use case.
go/private/rules/binary.bzl
Outdated
| version_dict[importpath] = version | ||
|
|
||
| # Write tab-separated file | ||
| lines = ["{}\t{}".format(imp, ver) for imp, ver in sorted(version_dict.items())] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can also do this lazily.
| buildSettingAppend("-compiler", cfg.compiler) | ||
| buildSettingAppend("-buildmode", cfg.buildMode) | ||
| if cfg.pgoProfilePath != "" { | ||
| buildSettingAppend("-pgo", cfg.pgoProfilePath) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to check that these paths aren't absolute. It would be very easy to accidentally destroy hermeticity that way. This also applies to other flags below.
| ) | ||
|
|
||
| func ModInfoData(info string) []byte { | ||
| return []byte(string(infoStart) + info + string(infoEnd)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a lot of back and forth conversion here and at the call site. Are the magic numbers UTF-8 strings? We should either choose []byte and append or string and + and convert at most once.
Implements Go 1.18+ runtime/debug.BuildInfo embedding in Bazel-built binaries with accurate dependency versions collected from Gazelle-generated package_info targets. This change introduces a buildinfo aspect that traverses the dependency graph and collects version metadata via the package_metadata common attribute (set by go_repository's default_package_metadata in REPO.bazel files). The aspect follows the bazel-contrib/supply-chain gather_metadata pattern. Features: - Collects real module versions (e.g., v1.2.3) from package_info metadata - Replaces (devel) sentinels with actual versions from package_info - Supports both package_metadata (Bazel 5.4+) and applicable_licenses (legacy) - Filters out internal monorepo packages to avoid invalid version strings - Embeds build settings (GOOS, GOARCH, CGO_ENABLED, etc.) in BuildInfo - Makes dependency information accessible via runtime/debug.ReadBuildInfo() Implementation details: - New buildinfo_aspect.bzl traverses all dependencies using attr_aspects=["*"] - Version map file aggregates importpath→version tuples and passes to linker - Link builder merges version map with buildinfo dependency list - Follows supply-chain memory optimization pattern for depset handling - Includes modinfo.go and buildinfo.go from Go stdlib for metadata generation Limitations: - Module checksums are not included as they are not available in package_metadata This enables tools like `go version -m` and OpenTelemetry instrumentation to properly identify dependencies in Bazel-built binaries. Fixes bazel-contrib#3090 Fixes bazel-contrib#4159 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
528017d to
2387572
Compare
|
@jsharpe Could you edit the commit messages to remove the |
Adds comprehensive tests for buildInfo metadata collection and refactors the implementation to address review comments on memory efficiency. Tests Added: - tests/core/buildinfo/metadata_test.go: Runtime test validating BuildInfo embedding for internal dependencies with multi-level dependency chains - tests/core/buildinfo/external_deps_test.go: Runtime test for external dependencies with real version metadata from package_info - Supporting test infrastructure including BUILD.bazel and README.md Refactoring Changes (addressing review comments): 1. buildinfo_aspect.bzl - Changed provider schema: - Split version_map into separate importpaths and metadata_providers depsets - Only traverse deps/embed attributes instead of all attributes (*) - Defer version matching to execution time to avoid quadratic memory usage - Extract empty provider as shared constant (_EMPTY_BUILDINFO_METADATA) - Remove applicable_licenses fallback (only Bazel 6+ supported) 2. archive.bzl - Removed quadratic aggregation: - Removed _buildinfo_deps tuple collection from GoArchiveData - BuildInfo metadata now collected only via aspect, not in archives - Eliminates O(n²) memory growth with dependency depth 3. link.bzl - Deferred materialization: - Updated to receive buildinfo_metadata provider instead of pre-computed tuples - Materialize depsets only once at link time - Extract versions from package_metadata providers on-demand 4. binary.bzl & rules/binary.bzl - Simplified metadata passing: - Pass BuildInfoMetadata provider directly instead of creating version_map file - Removed intermediate file generation and tuple aggregation The refactoring maintains the same functionality while significantly reducing memory usage for large dependency graphs by deferring depset materialization until the final link step. Tests validate: - BuildInfo is properly embedded in binaries - Main package path and Go version are included - All transitive dependencies appear in dependency list - Internal packages show "(devel)" as version - External dependencies with package_metadata show real versions 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
2387572 to
2aa512b
Compare
This commit addresses three test failures: 1. metadata_test - buildinfo was not being generated for binaries with only internal deps 2. external_deps_test - same issue as above 3. buildifier_test - Bazel files needed reformatting Changes: - importcfg.go: Generate buildinfo whenever buildinfoFile is provided, not just when deps list is non-empty. This ensures metadata is embedded even when all dependencies are internal (filtered out). - Reformat Bazel files with buildifier to fix formatting issues (sorted load statements, added blank lines per style guide) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
92484ab to
66c7a8a
Compare
Implements Go 1.18+ runtime/debug.BuildInfo embedding in Bazel-built binaries with accurate dependency versions collected from Gazelle-generated package_info targets.
This change introduces a buildinfo aspect that traverses the dependency graph and collects version metadata via the package_metadata common attribute (set by go_repository's default_package_metadata in REPO.bazel files). The aspect follows the bazel-contrib/supply-chain gather_metadata pattern.
Features:
Implementation details:
Limitations:
This enables tools like
go version -mand OpenTelemetry instrumentation to properly identify dependencies in Bazel-built binaries.Fixes #3090
Fixes #4159
🤖 Generated with Claude Code